packages/@webex/plugin-meetings/src/meeting/index.ts (2)

Line range hint 12-24: Consider adjusting the fee structure or discount policy.

The implementation of a flat $20 fee on discounted bills could negate the benefit of the discount, especially for smaller purchases or marginal loyalty tiers. This might lead to customer dissatisfaction, as the intent to reward loyalty paradoxically increases the bill.

Consider revising either the discount percentages or the flat fee application to better align with customer incentives.

Would you like assistance in generating a revised implementation?

---

Update requests library to version 2.32.3 to address security vulnerabilities

The current version 2.26.0 is vulnerable to:

• Session verification bypass (fixed in 2.32.0)
• Proxy-Authorization header leak (fixed in 2.31.0)

🔗 Analysis chain

Line range hint 6-6: Verify the latest stable versions and any security advisories

Ensure that the fixed version of the requests library is secure and free from vulnerabilities.

Run the following script to verify the fixed version of the requests library:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for security advisories and latest versions of the `requests` library.

# Check PyPI for latest versions
curl -s https://pypi.org/pypi/requests/json | jq '.info.version'

# Check for security advisories
gh api graphql -f query='
{
  securityVulnerabilities(first: 5, ecosystem: PIP, package: "requests") {
    nodes {
      advisory {
        summary
        severity
        publishedAt
      }
      vulnerableVersionRange
      firstPatchedVersion {
        identifier
      }
    }
  }
}'

Length of output: 1597

packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js (2)

Line range hint 3637-3732: Consider adding edge case tests for BRB functionality

Consider adding tests for the following edge cases:

it('should handle BRB enable when already enabled', async () => {
  meeting.isMultistream = true;
  meeting.meetingRequest.sendBrb = sinon.stub().resolves({body: 'test'});
  // Set initial state to enabled
  await meeting.beRightBack(true);
  // Try enabling again
  await meeting.beRightBack(true);
  assert.calledOnce(meeting.meetingRequest.sendBrb); 
});

it('should handle BRB disable when already disabled', async () => {
  meeting.isMultistream = true;
  meeting.meetingRequest.sendBrb = sinon.stub().resolves({body: 'test'});
  // Try disabling when already disabled
  await meeting.beRightBack(false);
  assert.calledOnce(meeting.meetingRequest.sendBrb);
});

---

Line range hint 8632-8713: Consider adding error handling tests

Consider adding tests for error handling scenarios:

it('should handle errors in BRB event processing', () => {
  const errorPayload = new Error('BRB processing failed');
  meeting.locusInfo.emit(
    {function: 'test', file: 'test'},
    LOCUSINFO.EVENTS.SELF_MEETING_BRB_CHANGED,
    errorPayload
  );
  
  // Verify error is handled gracefully
  assert.calledWith(
    TriggerProxy.trigger,
    meeting,
    {file: 'meeting/index', function: 'setUpLocusInfoSelfListener'},
    EVENT_TRIGGERS.MEETING_SELF_BRB_UPDATE,
    {payload: errorPayload}
  );
});

packages/@webex/plugin-meetings/src/meeting/request.type.ts (1)

15-20: LGTM! Consider adding JSDoc comments.

The BrbOptions type definition is well-structured with clear, required fields. Consider adding JSDoc comments to document the purpose of each field for better developer experience.

Example documentation:

/**
 * Options for updating a participant's "Be Right Back" status
 */
export type BrbOptions = {
  /** Whether the participant is in BRB state */
  enabled: boolean;
  /** URL of the locus where the BRB status should be updated */
  locusUrl: string;
  /** URL of the device sending the BRB update */
  deviceUrl: string;
  /** ID of the participant changing their BRB status */
  selfId: string;
};

packages/@webex/plugin-meetings/src/member/types.ts (1)

26-32: LGTM! Consider documenting the optional fields.

The ParticipantWithBrb type correctly uses optional chaining for both controls and brb properties, maintaining backward compatibility with existing participant objects that may not have these fields.

Consider adding documentation to explain when these fields might be undefined:

/**
 * Represents a participant with BRB (Be Right Back) status capabilities
 * Note: controls may be undefined for participants without BRB capabilities
 * or when the participant data is not fully loaded
 */
export type ParticipantWithBrb = {
  controls?: {
    /** BRB status - undefined when participant hasn't used BRB feature */
    brb?: {
      enabled: boolean;
    };
  };
};

packages/@webex/plugin-meetings/src/multistream/sendSlotManager.ts (1)

87-115: LGTM! Minor suggestion for error message consistency.

The implementation is well-structured with proper validation, error handling, and logging. The restriction to VideoMain media type is appropriate for BRB functionality.

Consider making the error message more consistent with other methods in the class:

-        `sendSlotManager cannot set source state override which media type is ${mediaType}`
+        `SendSlotManager cannot set source state override for media type ${mediaType}`

packages/@webex/plugin-meetings/src/member/util.ts (1)

54-60: LGTM! Consider enhancing the documentation.

The implementation is clean and follows the established patterns. The use of optional chaining provides good null safety.

Consider adding an example to the documentation:

  /**
   * Checks if the participant has the brb status enabled.
   *
   * @param {ParticipantWithBrb} participant - The locus participant object.
   * @returns {boolean} - True if the participant has brb enabled, false otherwise.
+  * @example
+  * const isBrb = MemberUtil.isBrb(participant); // returns true if participant is in BRB state
   */

packages/@webex/plugin-meetings/src/locus-info/selfUtils.ts (1)

165-167: Consider simplifying the undefined check.

The implementation is correct, but the undefined check might be redundant since isEqual would handle this case.

Consider simplifying to:

-SelfUtils.brbChanged = (previous, current) =>
-  !isEqual(previous?.brb, current?.brb) && current?.brb !== undefined;
+SelfUtils.brbChanged = (previous, current) =>
+  !isEqual(previous?.brb, current?.brb);

packages/@webex/plugin-meetings/src/meeting/index.ts (1)

Line range hint 4-4: Reminder: Address the TODO comment.

The TODO comment indicates that tests are missing for this function. Please ensure that the additional parameter change is thoroughly tested to confirm that it behaves as expected.

Do you want me to generate the unit testing code or open a GitHub issue to track this task?

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

#!/bin/bash
# Description: Verify endpoint pattern consistency
# Expected: Other participant control endpoints should follow similar pattern

rg "/${PARTICIPANT}/.+/${CONTROLS}" --type ts

#!/bin/bash
# Search for participant control endpoints in the meetings plugin
rg "/${PARTICIPANT}/.+/${CONTROLS}" packages/@webex/plugin-meetings/

#!/bin/bash
# Search for participant control endpoints in the meetings plugin
ast-grep --pattern 'const uri = `${locusUrl}/${PARTICIPANT}/${$_}/${CONTROLS}`'

#!/bin/bash
# Check for version inconsistencies in package.json files
fd -e package.json | xargs rg "@webex/internal-media-core.*2\.\d+\.\d+"

#!/bin/bash
# Check for version dependencies in package.json files
fd -e package.json | xargs rg "@webex/media-helpers.*workspace:|@webex/media-helpers.*\d+\.\d+\.\d+"

#!/bin/bash
# Check for any calling-related changes in the new version
gh api repos/webex/webex-js-sdk/compare/[email protected]@2.12.0 | jq -r '.files[].filename' | rg -i 'call'

#!/bin/bash
# Let's check how internal-media-core is used in the calling package
rg -A 5 "@webex/internal-media-core" packages/calling/

# Also check for any BRB (Background Replacement) related code in calling package
rg -i "background.?replacement|brb" packages/calling/

packages/@webex/plugin-meetings/src/meeting/index.ts (2)

3365-3378: Consider verifying payload for completeness.
It would be beneficial to ensure the BRB payload contains all necessary properties before dispatching the event. This offers better stability and avoids potential undefined/null references in downstream handlers.

---

3582-3625: Validate concurrency and error propagation.
The new beRightBack() method correctly checks if the meeting is multistream and if a media connection exists. However, consider adding concurrency guards if multiple overlapping calls to beRightBack() are possible. This helps prevent race conditions or unwanted overrides in status.

packages/@webex/plugin-meetings/src/constants.ts (3)

306-306: Adhere to naming conventions for new events.
The newly introduced MEETING_SELF_BRB_UPDATE event is consistent with existing naming but ensure it’s documented in your event schema references for clarity.

---

713-713: Document the new SELF_MEETING_BRB_CHANGED event.
Add developer documentation indicating how and when SELF_MEETING_BRB_CHANGED is emitted, and what data is contained in its payload, for easier maintainability.

---

Line range hint 9999-9999: Consider making BRB refresh threshold configurable.
MEETING_PERMISSION_TOKEN_REFRESH_THRESHOLD_IN_SEC and MEETING_PERMISSION_TOKEN_REFRESH_REASON appear to be static. Evaluate adding optional configuration parameters to allow flexible overrides across various deployments and compliance scenarios.

docs/samples/browser-plugin-meetings/app.js (2)

Line range hint 3739-3806: Consider extracting BRB column logic for better readability.

The BRB column integration looks good, but the conditional logic for rendering the BRB button/status could be extracted into a separate helper function to improve readability.

Consider this refactor:

-if (isUserSelf(member) && member.isInMeeting) {
-  td6.appendChild(createButton(member.isBrb ? 'Back to meeting' : 'Step away', toggleBrb, {id: 'brb-btn'}));
-} else {
-  td6.appendChild(createLabel(member.id, member.isBrb ? 'YES' : 'NO'));
-}

+function createBrbCell(member) {
+  if (isUserSelf(member) && member.isInMeeting) {
+    return createButton(member.isBrb ? 'Back to meeting' : 'Step away', toggleBrb, {id: 'brb-btn'});
+  }
+  return createLabel(member.id, member.isBrb ? 'YES' : 'NO');
+}
+
+td6.appendChild(createBrbCell(member));

---

3338-3355: Consider adding loading state during API call.

The implementation looks good but could benefit from showing a loading state while the API call is in progress to prevent multiple clicks.

Consider this enhancement:

async function toggleBrb() {
  const meeting = getCurrentMeeting();

  if (meeting) {
    const brbButton = document.getElementById('brb-btn');
+   const originalText = brbButton.innerText;
    const isBrbEnabled = brbButton.innerText === 'Step away';

    try {
+     brbButton.disabled = true;
+     brbButton.innerText = 'Please wait...';
      const result = await meeting.beRightBack(isBrbEnabled);
      console.log(`meeting.beRightBack(${isBrbEnabled}): success. Result: ${result}`);
    } catch (error) {
      console.error(`meeting.beRightBack({${isBrbEnabled}): error: `, error);
    } finally {
      localMedia?.microphoneStream?.setUserMuted(isBrbEnabled);
      localMedia?.cameraStream?.setUserMuted(isBrbEnabled);
+     brbButton.disabled = false;
+     brbButton.innerText = originalText;
    }
  }
}

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

packages/@webex/plugin-meetings/test/unit/spec/locus-info/index.js (2)

818-843: Consider Parametrizing the Test for Reusability
The sequence of "did not change" tests is correct. For maintainability, you might consider using a parameterized approach or loops for multiple unchanged states (e.g. brb = true and brb = false) to reduce code duplication in the future.

---

845-863: Test Undefined vs. Null BRB for Completeness
Currently, the test verifies behavior when brb is undefined. You may also consider adding a test case for brb = null to confirm consistent handling of other falsy or uninitialized states.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro